Skip to content

Conversation

@run1t
Copy link
Member

@run1t run1t commented Apr 23, 2016

Closes #23.

@SaturnusDJ
Copy link
Member

https://www.npmjs.com/package/emoji or https://www.npmjs.com/package/node-emoji no good?
Agree with @mrmayfield (#23 (comment)) that Twemoji doesn't look so good.

@stuwil stuwil self-assigned this Apr 23, 2016
@stuwil
Copy link
Contributor

stuwil commented Apr 23, 2016

Gonna merge this one in for now, as it's probably better than no emojis. Feel free to open a PR to swap the library.

return localStorage[cookieName];
};
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be here?

@SaturnusDJ
Copy link
Member

Not yet. That's in fact the code @run1t wrote to help me to create the profile menu entry in the new layout 3 branch.

@stuwil
Copy link
Contributor

stuwil commented Apr 23, 2016

Ok, cool. @run1t, can you remove that block from tinder-desktop.common.js then? Also, can we add the emoji filter to the Profile / Swipe bios?

@run1t
Copy link
Member Author

run1t commented Apr 23, 2016

Yep will do that

<link rel="stylesheet" href="css/sweet-alert.css"/>


<script src="https://twemoji.maxcdn.com/2/twemoji.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this via bower?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

twemoji it's really heavy (almost 300mb) that's why I use a CDN here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holy guacamole. The source, or the dist?!?!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the bower install twemojiso I guess the dist

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just the source for dev. Alright, let's leave it as a CDN resource for now...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also privacy implications, maybe we want to ship it locally and avoid outside dependencies?

@stuwil stuwil added this to the 0.1.0 milestone Apr 24, 2016
@stuwil
Copy link
Contributor

stuwil commented Apr 24, 2016

capture d ecran 2016-04-24 a 11 05 59

@stuwil stuwil removed their assignment Apr 25, 2016
@mayeaux mayeaux self-assigned this Apr 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants